[lldb] Support specifying a language for breakpoint conditions #147603
Conversation
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesLLDB breakpoint conditions take an expression that's evaluated using the language of the code where the breakpoint is located. Users have asked to have an option to tell it to evaluate the expression in a specific language. This is feature is especially helpful for Swift, for example for a condition based on the value in memory at an offset from a register. Such a condition is pretty difficult to write in Swift, but easy in C. This PR adds a new argument (-Y) to specify the language of the condition expression. We can't reuse the current -L option, since you might want to break on only Swift symbols, but run a C expression there as per the example above. rdar://146119507 Patch is 23.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147603.diff 14 Files Affected:
diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h
index b200a1e4893df..26a5e901a0d7e 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -397,16 +397,12 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>,
/// Set the breakpoint's condition.
///
/// \param[in] condition
- /// The condition expression to evaluate when the breakpoint is hit.
- /// Pass in nullptr to clear the condition.
- void SetCondition(const char *condition);
+ /// The condition to evaluate when the breakpoint is hit.
+ /// Pass in an empty condition to clear the condition.
+ void SetCondition(StopCondition condition);
- /// Return a pointer to the text of the condition expression.
- ///
- /// \return
- /// A pointer to the condition expression text, or nullptr if no
- // condition has been set.
- const char *GetConditionText() const;
+ /// Return the breakpoint condition.
+ const StopCondition &GetCondition() const;
// The next section are various utility functions.
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 66274e8825ee2..834d99bd2e0d5 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -129,14 +129,10 @@ class BreakpointLocation
///
/// \param[in] condition
/// The condition expression to evaluate when the breakpoint is hit.
- void SetCondition(const char *condition);
+ void SetCondition(StopCondition condition);
- /// Return a pointer to the text of the condition expression.
- ///
- /// \return
- /// A pointer to the condition expression text, or nullptr if no
- // condition has been set.
- const char *GetConditionText(size_t *hash = nullptr) const;
+ /// Return the breakpoint condition.
+ const StopCondition &GetCondition() const;
bool ConditionSaysStop(ExecutionContext &exe_ctx, Status &error);
diff --git a/lldb/include/lldb/Breakpoint/BreakpointOptions.h b/lldb/include/lldb/Breakpoint/BreakpointOptions.h
index 7bf545717422f..2f73473c07e62 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointOptions.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointOptions.h
@@ -12,6 +12,7 @@
#include <memory>
#include <string>
+#include "lldb/Breakpoint/StopCondition.h"
#include "lldb/Utility/Baton.h"
#include "lldb/Utility/Flags.h"
#include "lldb/Utility/StringList.h"
@@ -245,18 +246,15 @@ friend class Breakpoint;
const Baton *GetBaton() const;
// Condition
- /// Set the breakpoint option's condition.
+ /// Set the breakpoint stop condition.
///
/// \param[in] condition
- /// The condition expression to evaluate when the breakpoint is hit.
- void SetCondition(const char *condition);
+ /// The condition to evaluate when the breakpoint is hit.
+ void SetCondition(StopCondition condition);
- /// Return a pointer to the text of the condition expression.
- ///
- /// \return
- /// A pointer to the condition expression text, or nullptr if no
- // condition has been set.
- const char *GetConditionText(size_t *hash = nullptr) const;
+ /// Return the breakpoint condition.
+ const StopCondition &GetCondition() const;
+ StopCondition &GetCondition();
// Enabled/Ignore Count
@@ -390,9 +388,7 @@ friend class Breakpoint;
/// Thread for which this breakpoint will stop.
std::unique_ptr<ThreadSpec> m_thread_spec_up;
/// The condition to test.
- std::string m_condition_text;
- /// Its hash, so that locations know when the condition is updated.
- size_t m_condition_text_hash;
+ StopCondition m_condition;
/// If set, inject breakpoint condition into process.
bool m_inject_condition;
/// If set, auto-continue from breakpoint.
diff --git a/lldb/include/lldb/Breakpoint/StopCondition.h b/lldb/include/lldb/Breakpoint/StopCondition.h
new file mode 100644
index 0000000000000..cbb461b3ecbcc
--- /dev/null
+++ b/lldb/include/lldb/Breakpoint/StopCondition.h
@@ -0,0 +1,53 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_BREAKPOINT_STOPCONDITION_H
+#define LLDB_BREAKPOINT_STOPCONDITION_H
+
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+
+class StopCondition {
+public:
+ StopCondition(std::string text = "",
+ lldb::LanguageType language = lldb::eLanguageTypeUnknown)
+ : m_language(language) {
+ SetText(text);
+ }
+
+ operator bool() const { return !m_text.empty(); }
+
+ const std::string &GetText() const { return m_text; }
+
+ void SetText(std::string text) {
+ static std::hash<std::string> hasher;
+ m_text = std::move(text);
+ m_hash = hasher(text);
+ }
+
+ size_t GetHash() const { return m_hash; }
+
+ lldb::LanguageType GetLanguage() const { return m_language; }
+
+ void SetLanguage(lldb::LanguageType language) { m_language = language; }
+
+private:
+ /// The condition to test.
+ std::string m_text;
+
+ /// Its hash, so that locations know when the condition is updated.
+ size_t m_hash = 0;
+
+ /// The language for this condition.
+ lldb::LanguageType m_language = lldb::eLanguageTypeUnknown;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_BREAKPOINT_STOPCONDITION_H
diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp
index 397afc1f10f94..07c0a2ea907ba 100644
--- a/lldb/source/API/SBBreakpoint.cpp
+++ b/lldb/source/API/SBBreakpoint.cpp
@@ -275,7 +275,7 @@ void SBBreakpoint::SetCondition(const char *condition) {
if (bkpt_sp) {
std::lock_guard<std::recursive_mutex> guard(
bkpt_sp->GetTarget().GetAPIMutex());
- bkpt_sp->SetCondition(condition);
+ bkpt_sp->SetCondition(StopCondition(condition));
}
}
@@ -288,7 +288,7 @@ const char *SBBreakpoint::GetCondition() {
std::lock_guard<std::recursive_mutex> guard(
bkpt_sp->GetTarget().GetAPIMutex());
- return ConstString(bkpt_sp->GetConditionText()).GetCString();
+ return ConstString(bkpt_sp->GetCondition().GetText()).GetCString();
}
void SBBreakpoint::SetAutoContinue(bool auto_continue) {
diff --git a/lldb/source/API/SBBreakpointLocation.cpp b/lldb/source/API/SBBreakpointLocation.cpp
index b2d1da3927c6e..3f478be5a5e5e 100644
--- a/lldb/source/API/SBBreakpointLocation.cpp
+++ b/lldb/source/API/SBBreakpointLocation.cpp
@@ -160,7 +160,7 @@ void SBBreakpointLocation::SetCondition(const char *condition) {
if (loc_sp) {
std::lock_guard<std::recursive_mutex> guard(
loc_sp->GetTarget().GetAPIMutex());
- loc_sp->SetCondition(condition);
+ loc_sp->SetCondition(StopCondition(condition));
}
}
@@ -173,7 +173,7 @@ const char *SBBreakpointLocation::GetCondition() {
std::lock_guard<std::recursive_mutex> guard(
loc_sp->GetTarget().GetAPIMutex());
- return ConstString(loc_sp->GetConditionText()).GetCString();
+ return ConstString(loc_sp->GetCondition().GetText()).GetCString();
}
void SBBreakpointLocation::SetAutoContinue(bool auto_continue) {
diff --git a/lldb/source/API/SBBreakpointName.cpp b/lldb/source/API/SBBreakpointName.cpp
index 831260d44e8e7..0b588c38d5114 100644
--- a/lldb/source/API/SBBreakpointName.cpp
+++ b/lldb/source/API/SBBreakpointName.cpp
@@ -303,7 +303,7 @@ void SBBreakpointName::SetCondition(const char *condition) {
std::lock_guard<std::recursive_mutex> guard(
m_impl_up->GetTarget()->GetAPIMutex());
- bp_name->GetOptions().SetCondition(condition);
+ bp_name->GetOptions().SetCondition(StopCondition(condition));
UpdateName(*bp_name);
}
@@ -317,7 +317,8 @@ const char *SBBreakpointName::GetCondition() {
std::lock_guard<std::recursive_mutex> guard(
m_impl_up->GetTarget()->GetAPIMutex());
- return ConstString(bp_name->GetOptions().GetConditionText()).GetCString();
+ return ConstString(bp_name->GetOptions().GetCondition().GetText())
+ .GetCString();
}
void SBBreakpointName::SetAutoContinue(bool auto_continue) {
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp
index 8fc93cc8e0e51..be2cdc05de6f7 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -433,13 +433,13 @@ const char *Breakpoint::GetQueueName() const {
return m_options.GetThreadSpecNoCreate()->GetQueueName();
}
-void Breakpoint::SetCondition(const char *condition) {
- m_options.SetCondition(condition);
+void Breakpoint::SetCondition(StopCondition condition) {
+ m_options.SetCondition(std::move(condition));
SendBreakpointChangedEvent(eBreakpointEventTypeConditionChanged);
}
-const char *Breakpoint::GetConditionText() const {
- return m_options.GetConditionText();
+const StopCondition &Breakpoint::GetCondition() const {
+ return m_options.GetCondition();
}
// This function is used when "baton" doesn't need to be freed
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 7553315946043..90423d198755a 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -202,14 +202,13 @@ void BreakpointLocation::ClearCallback() {
GetLocationOptions().ClearCallback();
}
-void BreakpointLocation::SetCondition(const char *condition) {
- GetLocationOptions().SetCondition(condition);
+void BreakpointLocation::SetCondition(StopCondition condition) {
+ GetLocationOptions().SetCondition(std::move(condition));
SendBreakpointLocationChangedEvent(eBreakpointEventTypeConditionChanged);
}
-const char *BreakpointLocation::GetConditionText(size_t *hash) const {
- return GetOptionsSpecifyingKind(BreakpointOptions::eCondition)
- .GetConditionText(hash);
+const StopCondition &BreakpointLocation::GetCondition() const {
+ return GetOptionsSpecifyingKind(BreakpointOptions::eCondition).GetCondition();
}
bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
@@ -218,10 +217,9 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
std::lock_guard<std::mutex> guard(m_condition_mutex);
- size_t condition_hash;
- const char *condition_text = GetConditionText(&condition_hash);
+ StopCondition condition = GetCondition();
- if (!condition_text) {
+ if (!condition) {
m_user_expression_sp.reset();
return false;
}
@@ -230,19 +228,22 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
DiagnosticManager diagnostics;
- if (condition_hash != m_condition_hash || !m_user_expression_sp ||
+ if (condition.GetHash() != m_condition_hash || !m_user_expression_sp ||
!m_user_expression_sp->IsParseCacheable() ||
!m_user_expression_sp->MatchesContext(exe_ctx)) {
- LanguageType language = eLanguageTypeUnknown;
- // See if we can figure out the language from the frame, otherwise use the
- // default language:
- CompileUnit *comp_unit = m_address.CalculateSymbolContextCompileUnit();
- if (comp_unit)
- language = comp_unit->GetLanguage();
+ LanguageType language = condition.GetLanguage();
+ if (language == lldb::eLanguageTypeUnknown) {
+ // See if we can figure out the language from the frame, otherwise use the
+ // default language:
+ if (CompileUnit *comp_unit =
+ m_address.CalculateSymbolContextCompileUnit())
+ language = comp_unit->GetLanguage();
+ }
m_user_expression_sp.reset(GetTarget().GetUserExpressionForLanguage(
- condition_text, llvm::StringRef(), language, Expression::eResultTypeAny,
- EvaluateExpressionOptions(), nullptr, error));
+ condition.GetText(), llvm::StringRef(), language,
+ Expression::eResultTypeAny, EvaluateExpressionOptions(), nullptr,
+ error));
if (error.Fail()) {
LLDB_LOGF(log, "Error getting condition expression: %s.",
error.AsCString());
@@ -261,7 +262,7 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
return true;
}
- m_condition_hash = condition_hash;
+ m_condition_hash = condition.GetHash();
}
// We need to make sure the user sees any parse errors in their condition, so
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 2151c4c2dd204..99b85a11aa1e8 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -106,22 +106,22 @@ const char *BreakpointOptions::g_option_names[(
BreakpointOptions::BreakpointOptions(bool all_flags_set)
: m_callback(nullptr), m_baton_is_command_baton(false),
m_callback_is_synchronous(false), m_enabled(true), m_one_shot(false),
- m_ignore_count(0), m_condition_text_hash(0), m_inject_condition(false),
- m_auto_continue(false), m_set_flags(0) {
+ m_ignore_count(0), m_inject_condition(false), m_auto_continue(false),
+ m_set_flags(0) {
if (all_flags_set)
m_set_flags.Set(~((Flags::ValueType)0));
}
BreakpointOptions::BreakpointOptions(const char *condition, bool enabled,
- int32_t ignore, bool one_shot,
+ int32_t ignore, bool one_shot,
bool auto_continue)
: m_callback(nullptr), m_baton_is_command_baton(false),
m_callback_is_synchronous(false), m_enabled(enabled),
- m_one_shot(one_shot), m_ignore_count(ignore), m_condition_text_hash(0),
+ m_one_shot(one_shot), m_ignore_count(ignore), m_condition(condition),
m_inject_condition(false), m_auto_continue(auto_continue) {
m_set_flags.Set(eEnabled | eIgnoreCount | eOneShot | eAutoContinue);
if (condition && *condition != '\0') {
- SetCondition(condition);
+ SetCondition(StopCondition(condition));
}
}
@@ -135,8 +135,7 @@ BreakpointOptions::BreakpointOptions(const BreakpointOptions &rhs)
m_auto_continue(rhs.m_auto_continue), m_set_flags(rhs.m_set_flags) {
if (rhs.m_thread_spec_up != nullptr)
m_thread_spec_up = std::make_unique<ThreadSpec>(*rhs.m_thread_spec_up);
- m_condition_text = rhs.m_condition_text;
- m_condition_text_hash = rhs.m_condition_text_hash;
+ m_condition = rhs.m_condition;
}
// BreakpointOptions assignment operator
@@ -151,8 +150,7 @@ operator=(const BreakpointOptions &rhs) {
m_ignore_count = rhs.m_ignore_count;
if (rhs.m_thread_spec_up != nullptr)
m_thread_spec_up = std::make_unique<ThreadSpec>(*rhs.m_thread_spec_up);
- m_condition_text = rhs.m_condition_text;
- m_condition_text_hash = rhs.m_condition_text_hash;
+ m_condition = rhs.m_condition;
m_inject_condition = rhs.m_inject_condition;
m_auto_continue = rhs.m_auto_continue;
m_set_flags = rhs.m_set_flags;
@@ -187,13 +185,11 @@ void BreakpointOptions::CopyOverSetOptions(const BreakpointOptions &incoming)
if (incoming.m_set_flags.Test(eCondition))
{
// If we're copying over an empty condition, mark it as unset.
- if (incoming.m_condition_text.empty()) {
- m_condition_text.clear();
- m_condition_text_hash = 0;
+ if (!incoming.m_condition) {
+ m_condition = StopCondition();
m_set_flags.Clear(eCondition);
} else {
- m_condition_text = incoming.m_condition_text;
- m_condition_text_hash = incoming.m_condition_text_hash;
+ m_condition.SetText(incoming.m_condition.GetText());
m_set_flags.Set(eCondition);
}
}
@@ -363,8 +359,8 @@ StructuredData::ObjectSP BreakpointOptions::SerializeToStructuredData() {
m_ignore_count);
if (m_set_flags.Test(eCondition))
options_dict_sp->AddStringItem(GetKey(OptionNames::ConditionText),
- m_condition_text);
-
+ m_condition.GetText());
+
if (m_set_flags.Test(eCallback) && m_baton_is_command_baton) {
auto cmd_baton =
std::static_pointer_cast<CommandBaton>(m_callback_baton_sp);
@@ -464,29 +460,21 @@ bool BreakpointOptions::GetCommandLineCallbacks(StringList &command_list) {
return true;
}
-void BreakpointOptions::SetCondition(const char *condition) {
- if (!condition || condition[0] == '\0') {
- condition = "";
+void BreakpointOptions::SetCondition(StopCondition condition) {
+ if (!condition)
m_set_flags.Clear(eCondition);
- }
else
m_set_flags.Set(eCondition);
- m_condition_text.assign(condition);
- std::hash<std::string> hasher;
- m_condition_text_hash = hasher(m_condition_text);
+ m_condition = std::move(condition);
}
-const char *BreakpointOptions::GetConditionText(size_t *hash) const {
- if (!m_condition_text.empty()) {
- if (hash)
- *hash = m_condition_text_hash;
-
- return m_condition_text.c_str();
- }
- return nullptr;
+const StopCondition &BreakpointOptions::GetCondition() const {
+ return m_condition;
}
+StopCondition &BreakpointOptions::GetCondition() { return m_condition; }
+
const ThreadSpec *BreakpointOptions::GetThreadSpecNoCreate() const {
return m_thread_spec_up.get();
}
@@ -555,10 +543,10 @@ void BreakpointOptions::GetDescription(Stream *s,
s->GetIndentLevel());
}
}
- if (!m_condition_text.empty()) {
+ if (m_condition) {
if (level != eDescriptionLevelBrief) {
s->EOL();
- s->Printf("Condition: %s\n", m_condition_text.c_str());
+ s->Printf("Condition: %s\n", m_condition.GetText().c_str());
}
}
}
@@ -652,5 +640,5 @@ void BreakpointOptions::Clear()
m_baton_is_command_baton = false;
m_callback_is_synchronous = false;
m_enabled = false;
- m_condition_text.clear();
+ m_condition = StopCondition();
}
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 4631c97bf50ae..b299d27536ee8 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -71,7 +71,7 @@ class lldb_private::BreakpointOptionGroup : public OptionGroup {
case 'c':
// Normally an empty breakpoint condition marks is as unset. But we need
// to say it was passed in.
- m_bp_opts.SetCondition(option_arg.str().c_str());
+ m_bp_opts.GetCondition().SetText(option_arg.str());
m_bp_opts.m_set_flags.Set(BreakpointOptions::eCondition);
break;
case 'C':
@@ -153,6 +153,14 @@ class lldb_private::BreakpointOptionGroup : public OptionGroup {
m_bp_opts.GetThreadSpec()->SetIndex(thread_index);
}
} break;
+ case 'Y': {
+ LanguageType language = Language::GetLanguageTypeFromString(option_arg);
+ if (language != eLanguageTypeUnknown)
+ m_bp_opts.GetCondition().SetLanguage(language);
+ else
+ error = Status::FromError(CreateOptionParsingError(
+ option_arg, short_option, long_option, "invalid language"));
+ } break;
default:
llvm_unreachable("Unimplemented option");
}
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 5327647d65cc4..d32dbdc1d3533 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -92,6 +92,12 @@ let Command = "breakpoint modify" in {
def breakpoint_modify_condition : Option<"condition", "c">, Group<1>,
Arg<"Expression">, Desc<"The breakpoint stops only if this condition "
"expression evaluates to true.">;
+ def breakpoint_modify_condition_langauge
+ : Option<"condition-language", "Y">,
+ Group<1>,
+ Arg<"Language">,
+ Desc<"Specifies the Language to use when executing the breakpoint's "
+ "condition expression.">;
def breakpoint_modify_auto_continue : Option<"auto-continue", "G">, Group<1>,
Arg<"Boolean">,
Desc<"The breakpoint will auto-continue after running its commands.">;
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 3160446ae1d17..19f89b8246926 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -465,7 +465,7 @@ class StopInfoBreakpoint : public StopInfo {
// should stop, t...
[truncated]
|
JDevlieghere
force-pushed
the
rdar146119507
branch
from
fac9c09 to
67ce4cd
Compare
|
✅ With the latest revision this PR passed the Python code formatter. |
JDevlieghere
force-pushed
the
rdar146119507
branch
from
67ce4cd to
32d31ba
Compare
|
Semi-related: I wonder if we should have a way to set the language from within the condition expression itself. E.g. a We might also want to support something similar, e.g., in the watch panel where users can also only enter expressions. E.g., CodeLLDB (the other DAP implementation besides lldb-dap) provides a similar mechanism which also covers breakpoint conditions, documented over here: https://github.com/vadimcn/codelldb/blob/master/MANUAL.md#expressions I wonder if those "expression prefixes" should be part of lldb / the new upcoming DIL language itself or layered on top of it by lldb-dap... |
| @@ -173,7 +173,7 @@ const char *SBBreakpointLocation::GetCondition() { | |||
|
|
|||
| std::lock_guard<std::recursive_mutex> guard( | |||
| loc_sp->GetTarget().GetAPIMutex()); | |||
| return ConstString(loc_sp->GetConditionText()).GetCString(); | |||
| return ConstString(loc_sp->GetCondition().GetText()).GetCString(); | |||
There was a problem hiding this comment.
I know this code was like this before but why do we construct an intermediate ConstString here? Should we remove it?
There was a problem hiding this comment.
We use ConstString at the SB API level to make sure the pointers we had out point into the string pool and remain valid. That way clients don't have to worry about their lifetime which is really an implementation detail.
| """Exercise breakpoint condition with 'breakpoint modify -c <expr> id'.""" | ||
| exe = self.getBuildArtifact("a.out") | ||
| self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) | ||
|
|
||
| cmd_args = "-c 'val == 3'" |
There was a problem hiding this comment.
This seems like it would still pass if the -Y option were to be ignored, since it's a valid conditional in Swift and C.
There was a problem hiding this comment.
You're right, I changed the condition to use nullptr so that it fails to parse if the language is wrong.
JDevlieghere
force-pushed
the
rdar146119507
branch
from
32d31ba to
9e4ce45
Compare
LLDB breakpoint conditions take an expression that's evaluated using the language of the code where the breakpoint is located. Users have asked to have an option to tell it to evaluate the expression in a specific language. This is feature is especially helpful for Swift, for example for a condition based on the value in memory at an offset from a register. Such a condition is pretty difficult to write in Swift, but easy in C. This PR adds a new argument (-Y) to specify the language of the condition expression. We can't reuse the current -L option, since you might want to break on only Swift symbols, but run a C expression there as per the example above. rdar://146119507
JDevlieghere
force-pushed
the
rdar146119507
branch
from
9e4ce45 to
7aa2ae6
Compare
The DIL is only used for "variable path expressions". It's a C-ish but not language accurate way of dialing into static variables (and in the new revision to do simple math & casts). The DIL language is orthogonal to the expression evaluators, which are intended to be "language accurate". So a "language specification" is unnecessary by the time the condition text gets to the expression evaluator. And we don't want the "language accurate evaluators" to have to deal with this likely non-language compliant construct. That's relevant because condition expressions are run using the expression evaluator, not the DIL. So changes to it don't affect how conditions are handled. I think we do want to do this with options, both because that's the way the lldb command line works and because options make it easy to construct special purpose aliases. I'm not altogether opposed to adding something that the 'condition option value parser' strips off and uses to set the -Y option. But it's always a little weird to have two ways to do the same thing. This does seem more like something that should get added & handled by DAP. |
|
Should it be an error to specify -Y but not -c? |
Specifying |
|
Thinking about that some more, it seems reasonable to do: (lldb) commmand alias swbp break set -Y swift -L swift and then use that for all your swift breakpoints. It would then make sense to proactively set the condition language to swift, regardless of whether you're going to add a condition every time. Even a warning would be annoying for this usage. I don't think we need that. |
| @@ -129,14 +129,10 @@ class BreakpointLocation | |||
| /// | |||
| /// \param[in] condition | |||
| /// The condition expression to evaluate when the breakpoint is hit. | |||
There was a problem hiding this comment.
You removed the word "expression" from the help in Breakpoint.h, it's confusing to have these two differ.
There was a problem hiding this comment.
I dropped it because the condition is more than just the expression text. Removed it here too for consistency. In all the user-facing strings we just use "breakpoint condition".
| @@ -153,6 +153,14 @@ class lldb_private::BreakpointOptionGroup : public OptionGroup { | |||
| m_bp_opts.GetThreadSpec()->SetIndex(thread_index); | |||
| } | |||
| } break; | |||
| case 'Y': { | |||
| LanguageType language = Language::GetLanguageTypeFromString(option_arg); | |||
There was a problem hiding this comment.
Since this is supposed to be a language that we can use to run the condition expression, you should check the language you got from the option against Language::GetLanguagesSupportingTypeSystemsForExpressions(). It would be wrong to allow people to set a language for a condition expression to one for which we can't evaluate expressions.
|
Other than validating that the condition expression language is one for which we can run expressions, this LGTM. It's also nice to bundle this into a class, so when the DIL can express conditions we can add the ability to specify either a DIL or a language expression. |
LLDB breakpoint conditions take an expression that's evaluated using the language of the code where the breakpoint is located. Users have asked to have an option to tell it to evaluate the expression in a specific language.
This is feature is especially helpful for Swift, for example for a condition based on the value in memory at an offset from a register. Such a condition is pretty difficult to write in Swift, but easy in C.
This PR adds a new argument (-Y) to specify the language of the condition expression. We can't reuse the current -L option, since you might want to break on only Swift symbols, but run a C expression there as per the example above.
rdar://146119507